resolve $BRANCH as current branch name in branch_name_template#40
Conversation
|
I realized my formatter created unnecessary changes. Updating PR. |
eac3959 to
e23564e
Compare
ZolotukhinM
left a comment
There was a problem hiding this comment.
Thanks, that's a nice addition! I have one comment, but otherwise this looks good!
| @cache | ||
| def get_branch_name_base(branch_name_template: str): | ||
| username = get_gh_username() | ||
| current_branch_name = get_current_branch_name() |
There was a problem hiding this comment.
Calling it here is slightly error-prone given how many manipulations the tool does with branches. E.g. I'm concerned that if we don't call it at the right moment, it would return an internal stack branch name rather than the actual name of the user branch.
To prevent this (and make use of the fact that get_branch_name_base is memoized), let's explicitly call get_branch_name_base here:
https://github.com/modularml/stack-pr/blob/5220b763f50b4feb449b0ad82cd24f6a40f45b88/src/stack_pr/cli.py#L1385
There was a problem hiding this comment.
Updated the PR! Let me know if the updated approach is what you are expecting. Feel free to merge it after (I don't have access to do so).
There was a problem hiding this comment.
Yes, thanks, I'll merge it for you!
One thing I realized though is this can potentially be a source of subtle problems - e.g. if I create a stack while being on branch aaa, then switch to a branch bbb and create another stack, and then decide to merge the two stacks into one. I won't be surprised if something in the current implementation would break if PRs in the same stack use different branch name patterns - that's probably a rare situation and we can address the issues as they arrive.
e23564e to
5bcd6de
Compare
5bcd6de to
3acd32e
Compare
This PR adds the ability for someone to use $BRANCH in the branch_name_template which would correspond to the current branch name.